release: v0.9.1 — Stability (architecture + code quality) - #203

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/components/dashboard-container.tsx`:
- Around line 56-60: DashboardContainer lost a top-level onNavigateToPage prop
so CardContainer only gets actions?.onNavigateToPage and view-mode callers that
don't pass actions break navigation; restore onNavigateToPage into
DashboardContainerProps (add onNavigateToPage?: (page: DashboardPage) => void),
accept and forward that prop from DashboardContainer to CardContainer (use
onNavigateToPage prop as fallback when actions?.onNavigateToPage is undefined),
and ensure the caller that renders DashboardContainer without actions continues
to work without changing callers.

In `@app/src/components/widget-editor-modal.tsx`:
- Around line 813-875: The store-seeding calls currently run in passive
useEffect hooks causing a one-render staleness for sub-editors; replace those
initial useEffect calls that call useWidgetEditorStore.setState (the three small
ones and the larger stylingRules/actionRules/... sync) with useLayoutEffect so
the store is updated synchronously before paint, keep the existing
syncingFromStore ref logic to avoid feedback loops, and ensure any places that
update the modal snapshot (where chartType, connectionId, query,
availableFields, parameterSuggestions, stylingRules, etc. are set) call
useWidgetEditorStore.setState synchronously (or migrate to making
useWidgetEditorStore the single source of truth) so
QueryEditorPanel/useConnectionSchema observe the updated values immediately.

In `@app/src/components/widget-editor/styling-rules-editor.tsx`:
- Around line 47-56: Replace the local NULL_OPS/STRING_OPS-driven logic in
inputType and the single-value path with metadata from the shared operator
registry returned by getOperatorGroups()/OPERATOR_GROUPS: look up the chosen
operator's metadata (its declared input kind/type) and base rendering (text vs
number vs null vs between) on that instead of hardcoded sets; remove
NULL_OPS/STRING_OPS usage, ensure equality operators (==, !=) use the operator
metadata so string equality renders text inputs when supported by
component/src/charts/styling-rule.ts, and make the "between" control pass the
explicit numeric inputType in the single-value code path when the operator
metadata requires numeric inputs.

In `@app/src/components/widget-editor/use-accordion-crud.ts`:
- Around line 59-61: The new accordion id is being opened optimistically in
addItem() but prevIdsRef.current isn't updated until items prop changes, so
computeOpenItems(prevIdsRef.current, currentIds, prev) sees the id as newly
added and returns it twice; fix by updating prevIdsRef.current immediately when
you optimistically open the new item (e.g., append newItem.id to
prevIdsRef.current) before calling setOpenItems, or alternatively ensure
computeOpenItems deduplicates ids, so references to setOpenItems,
computeOpenItems, prevIdsRef, and addItem are adjusted accordingly to prevent
the duplicate id.

In `@app/src/stores/widget-editor-store.ts`:
- Around line 325-369: The advanced-rules branch in buildClickAction currently
returns s.actionRules without validation; update buildClickAction so that before
returning when s.actionRules.length > 0 it iterates over s.actionRules and
validates each rule the same way the legacy branch does: for rules with type
"set-parameter" or "set-parameter-and-navigate" ensure parameterName.trim() is
non-empty and sourceField defaults to parameterName; for rules with type
"navigate-to-page" or "set-parameter-and-navigate" ensure targetPageId is
present and is included in (layout?.pages ?? []).map(p => p.id); also ensure
clickableColumns is set only if non-empty (otherwise undefined); if any rule
fails validation return undefined, otherwise return the validated rules object
(type from first rule, rules array, and clickableColumns as before).

In `@connection/src/generalized/ConnectorError.ts`:
- Around line 107-108: The wrapError helper currently converts non-Error inputs
to a string with String(err), which yields "[object Object]" for object-shaped
errors and loses any message property; update wrapError (in ConnectorError.ts)
to detect non-Error objects and, if they have a truthy string-valued message
property (e.g., err && typeof err === "object" && typeof err.message ===
"string"), use that message, otherwise fall back to JSON.stringify(err) (with
safe fallback to String(err)) before constructing new ConnectorError(message,
type, err).
- Around line 1-4: Errors thrown by drivers are being passed through unwrapped;
call the existing wrapError helper wherever errors are caught and before
invoking callbacks.onFail or rethrowing so consumers always receive
ConnectorError. Specifically, update the catch blocks that currently call
callbacks.onFail with raw errors (the PostgreSQL error handler and the Neo4j
error handler) to pass wrapError(error) instead, and modify
PostgresAuthenticationModule and Neo4jConnectionModule catch/rethrow sites
(e.g., the methods that currently rethrow driver errors) to throw
wrapError(error) or throw a new ConnectorError created by wrapError (preserving
the original as the cause if supported). Ensure every catch path that surfaces
errors to callers or callbacks uses wrapError so the contract is upheld.
- Around line 77-79: The mapping in ConnectorError.ts wrongly classifies
Postgres error code "3D000" (invalid_catalog_name) as AUTHENTICATION; remove
"3D000" from the array checked for ConnectorErrorType.AUTHENTICATION (the code
that checks if (["28P01", "28000", "28001", "3D000"].includes(code)) and returns
ConnectorErrorType.AUTHENTICATION) and instead include "3D000" in the set/branch
that returns ConnectorErrorType.CONNECTION so that ConnectorErrorType reflects a
missing/invalid database rather than an auth failure; update any related tests
or comments referencing this mapping.

---

Nitpick comments:
In `@app/src/components/card-container.tsx`:
- Around line 39-42: The helper supportsColumnMapping should be a TypeScript
type guard so the downstream cast to the overlay-specific chart union is safe:
change the signature of supportsColumnMapping(type: string) to a type predicate
(e.g. type is ChartTypeWithColumnMapping or type is "bar" | "line" | "pie") and
keep the runtime check using getChartConfig(type)?.supportsColumnMapping ===
true; update any call sites that rely on the narrowed type so the compiler
understands the guard, and reference the ChartConfig.supportsColumnMapping
property, getChartConfig, and the supportsColumnMapping function in your
changes.

In `@app/src/stores/widget-editor-store.ts`:
- Around line 19-44: Extract the duplicated types and function into a non-UI
helper module (e.g., under app/src/lib/) and import it from both
widget-editor-store and parameter-config-section to avoid drift: move
ParamUIType, DateSubType, and reverseParamTypeMapping into the new shared file,
export them, then replace the local declarations in widget-editor-store (and
remove the duplicates in parameter-config-section.tsx) with imports of those
exported symbols so both places use the single canonical mapping.

In `@connection/__tests__/connector-error.test.ts`:
- Around line 96-103: Add an assertion in the "wraps Neo4j error correctly" test
to verify the wrapped.message is not a stringified object; specifically, after
calling wrapError(raw, "neo4j") and the existing assertions on ConnectorError
and ConnectorErrorType, assert that wrapped.message contains the raw.message
(and optionally raw.code) so regressions to "[object Object]" are caught —
update the test around the wrapError call in connector-error.test.ts to include
expect(wrapped.message).toContain(raw.message) (and/or
expect(wrapped.message).toContain(raw.code)).
- Around line 1-7: Move the test file into the package-local test folder so it
sits next to the module: relocate connection/__tests__/connector-error.test.ts
to connection/src/generalized/__tests__/connector-error.test.ts; after moving,
update the import path that currently points to
"../src/generalized/ConnectorError" to the module-local relative path
"../ConnectorError" so the imports (ConnectorError, ConnectorErrorType,
detectNeo4jErrorType, detectPostgresErrorType, wrapError) resolve correctly from
the new location and the test lives in __tests__/ next to the source.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/components/card-container.tsx`:
- Around line 191-193: The current check uses
supportsColumnMapping(widget.chartType) and then force-casts chartConfig.type to
"bar" | "line" | "pie" before passing to ColumnMappingOverlay, which breaks type
safety for new registry entries; update the guard so the value is actually
narrowed to the overlay-supported union — either (A) change
supportsColumnMapping to perform a type-narrowing check (a user-defined type
guard) so calling code can safely treat chartConfig.type as "bar"|"line"|"pie",
or (B) add an explicit runtime check against the allowed set (e.g.,
chartConfig.type === "bar" || "line" || "pie") before casting; apply the same
fix at the other two call sites and use the symbols supportsColumnMapping,
chartConfig.type, ColumnMappingOverlay, and onWidgetSettingsChange to locate and
update the checks.
- Around line 133-170: The gating for whether to build queryInput is currently
hardcoded via isParameterWidget/isFormWidget/isContentOnly; replace that with
the chart registry metadata (e.g., use the registry lookup like
getChartRegistryEntry(widget.chartType) or ChartRegistry[widget.chartType] and
its requiresQuery flag) so widgets with requiresQuery: false no longer run
useWidgetQuery; update the queryInput expression to check previewData ||
!requiresQuery (instead of the three chartType booleans) and remove or derive
isParameterWidget/isFormWidget/isContentOnly from the registry-based
requiresQuery where needed.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@component/src/components/composed/query-editor.tsx`:
- Around line 296-300: The teardown for the editor is leaving a stale DOM
property and attribute; update destroyEditor() to remove the exposed test handle
and readiness flag by deleting containerRef.current.__cmView (use the same any
cast pattern as in initEditor for containerRef) and removing the
"data-editor-ready" attribute on containerRef.current when tearing down the
EditorView referenced by viewRef.current; ensure you check containerRef.current
exists before clearing both the property and attribute to avoid null access.

---

Nitpick comments:
In `@app/e2e/fixtures.ts`:
- Around line 122-139: The inline callback passed to cmContainer.evaluate uses
(el as any).__cmView and relies on properties/methods like state.readOnly,
state.doc.length and dispatch; replace the ad-hoc any with a small local
interface (e.g. CodeMirrorContainer) that declares the hook shape (state: {
readOnly: boolean; doc: { length: number; toString(): string } }, dispatch: (tr:
any) => void) and cast to that type instead of any where __cmView is read in the
evaluate callback and the similar helper at lines 146-154 so TypeScript strict
mode can validate those members.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@component/src/components/composed/query-editor.tsx`:
- Around line 228-229: Add explicit rationale comments for the
`@typescript-eslint/no-explicit-any` suppressions used when attaching E2E
handles to the DOM: for the instance where you delete (containerRef.current as
any).__cmView (referencing containerRef and __cmView) and the other suppression
around the non-standard E2E handle later in this file; place a one-line comment
immediately above each `// eslint-disable-next-line
`@typescript-eslint/no-explicit-any`` that explains that `__cmView` (and the other
handle) are non-standard test-only properties added to the DOM for E2E testing
and cannot be strongly typed, so `any` is required to access/delete them.

In `@connection/__tests__/connector-error.test.ts`:
- Around line 1-7: Test file is located centrally and should be colocated with
the module under test; move the test from
connection/__tests__/connector-error.test.ts into the same package as the
ConnectorError module under a module-local __tests__ directory. Update the
import path to reference the local source (e.g., import from "./ConnectorError"
or "../ConnectorError" depending on module layout) so tests exercise
ConnectorError, ConnectorErrorType, detectNeo4jErrorType,
detectPostgresErrorType, and wrapError directly alongside the implementation.
- Around line 101-116: The current unit tests only exercise wrapError directly;
add module-level tests that trigger the catch paths in Neo4jConnectionModule and
PostgresConnectionModule and assert that callbacks.onFail receives a
ConnectorError (not a raw Error). Specifically, write tests that instantiate or
mock Neo4jConnectionModule and PostgresConnectionModule, cause their operation
to throw (e.g., mock the driver/session/query to throw a connection/timeout
error), spy/mock callbacks.onFail, and assert the argument is an instance of
ConnectorError with the expected ConnectorErrorType; reference wrapError,
callbacks.onFail, Neo4jConnectionModule, and PostgresConnectionModule to locate
the code paths to test.

---

Nitpick comments:
In `@app/src/stores/widget-editor-store.ts`:
- Around line 380-381: The code builds validPageIds as an array and uses
Array.includes for lookup, causing inconsistency with the earlier advanced-rules
branch that uses a Set and Set.has; change validPageIds to a Set of IDs (e.g.,
new Set((layout?.pages ?? []).map(p => p.id))) and replace the includes check
with validPageIds.has(targetPageId) so both branches use O(1) Set lookups;
update any variable name if helpful (e.g., validPageIdSet) to reflect the type.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@connection/src/generalized/ConnectorError.ts`:
- Around line 35-37: The code currently assumes err.message and err.code are
strings which can cause classifiers to throw; in detectNeo4jErrorType and
detectPostgresErrorType replace the unsafe casts that set msg and code with
explicit runtime type guards so msg and code are guaranteed strings before any
string operations—use expressions like typeof e.message === "string" ? e.message
: "" and typeof e.code === "string" ? e.code : "" when initializing msg and code
(these variables are used in detectNeo4jErrorType and detectPostgresErrorType)
to prevent non-string values from reaching
msg.includes()/msg.startsWith()/msg.toLowerCase()/msg.includes().

---

Nitpick comments:
In `@app/src/stores/widget-editor-store.ts`:
- Around line 253-257: In setChartType, avoid multiple set() calls that can
race; instead compute the new state in one atomic update by calling set once:
derive chartOptions via getChartDefaults(t), compute clickActionEnabled as
chartSupportsClickAction(t) ? current or true/false as needed, and
stylingEnabled as chartSupportsStyling(t) ? current or true/false as needed,
then call set({ chartType: t, chartOptions, clickActionEnabled, stylingEnabled
}). Reference setChartType, set, getChartDefaults, chartSupportsClickAction,
chartSupportsStyling, and the state keys chartType, chartOptions,
clickActionEnabled, stylingEnabled when making the change.